Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 6 minutes and 39 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdded Perplexity AI provider support to the llmproxy system. Includes new provider implementation with URL resolver and request enricher configuration, comprehensive unit tests, and integration into the basic example. Updated go.mod to explicitly declare the OpenTelemetry trace dependency. Changes
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
providers/perplexity/provider_test.go (1)
81-90: Strengthen empty-key test with header assertion.
TestNew_EmptyKeyshould also verify noAuthorizationheader is injected when the API key is empty.✅ Suggested test extension
func TestNew_EmptyKey(t *testing.T) { provider, err := New("") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ if provider.Name() != "perplexity" { t.Errorf("Name = %q, want %q", provider.Name(), "perplexity") } + + req := httptest.NewRequest("POST", "https://api.perplexity.ai/v1/sonar", nil) + if err := provider.RequestEnricher().Enrich(req, llmproxy.BodyMetadata{}, nil); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got := req.Header.Get("Authorization"); got != "" { + t.Errorf("Authorization = %q, want empty", got) + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@providers/perplexity/provider_test.go` around lines 81 - 90, Extend TestNew_EmptyKey to assert that no Authorization header is sent when the API key is empty: create an httptest.Server whose handler fails the test if r.Header.Get("Authorization") is non-empty, then create the provider with New(""), make a request to that server using the provider's HTTP client (use provider.client or provider.httpClient if available, otherwise construct the request the provider would send and call http.DefaultClient.Do) and verify the server handler was invoked without an Authorization header; keep the existing Name() assertion intact.providers/perplexity/resolver.go (1)
13-15: The nil check suggestion is defensive programming only—not a bug fix.The Perplexity
Resolveris exclusively instantiated viaNewResolver(), which parses the base URL and returns an error if parsing fails. TheProvider.New()constructor propagates this error, meaningResolve()is never reachable with a nil or uninitialized receiver in practice. Other resolver implementations in the codebase (openai_compatible, googleai, bedrock, azure) use the same pattern without nil guards.If defensive nil checks are desired for robustness, consider applying them consistently across all resolver implementations rather than in isolation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@providers/perplexity/resolver.go` around lines 13 - 15, Do not add a solitary nil-receiver guard inside Resolver.Resolve; keep the current implementation returning r.BaseURL.JoinPath("v1", "sonar"), nil as-is because Resolver is constructed via NewResolver() and Provider.New() already enforces valid BaseURL; if you do want defensive nil checks, apply the same pattern consistently to all resolver types (e.g., Resolver.Resolve, openai_compatible resolver, googleai resolver, bedrock resolver, azure resolver) and implement identical nil checks and error returns across their Resolve methods so behavior is uniform.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@providers/perplexity/provider_test.go`:
- Around line 81-90: Extend TestNew_EmptyKey to assert that no Authorization
header is sent when the API key is empty: create an httptest.Server whose
handler fails the test if r.Header.Get("Authorization") is non-empty, then
create the provider with New(""), make a request to that server using the
provider's HTTP client (use provider.client or provider.httpClient if available,
otherwise construct the request the provider would send and call
http.DefaultClient.Do) and verify the server handler was invoked without an
Authorization header; keep the existing Name() assertion intact.
In `@providers/perplexity/resolver.go`:
- Around line 13-15: Do not add a solitary nil-receiver guard inside
Resolver.Resolve; keep the current implementation returning
r.BaseURL.JoinPath("v1", "sonar"), nil as-is because Resolver is constructed via
NewResolver() and Provider.New() already enforces valid BaseURL; if you do want
defensive nil checks, apply the same pattern consistently to all resolver types
(e.g., Resolver.Resolve, openai_compatible resolver, googleai resolver, bedrock
resolver, azure resolver) and implement identical nil checks and error returns
across their Resolve methods so behavior is uniform.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6d66e909-fa15-40b5-b6f5-d282a14c6688
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (5)
examples/basic/main.gogo.modproviders/perplexity/provider.goproviders/perplexity/provider_test.goproviders/perplexity/resolver.go
📜 Review details
🔇 Additional comments (6)
go.mod (2)
5-6: LGTM: Correctly promotes trace package to direct dependency.The change appropriately declares
go.opentelemetry.io/otel/traceas a direct dependency, which aligns with Go module best practices since the package is directly imported and used inexamples/basic/main.go.
5-5: The OpenTelemetry trace package version v1.43.0 is valid and has no known security advisories.The version exists in the Go module proxy (released 2026-04-03) with 0 reported vulnerabilities.
examples/basic/main.go (2)
116-123: Perplexity provider registration looks correct.
PERPLEXITY_API_KEY-gated initialization, fail-fast constructor error handling, and registry wiring are all solid.
179-179: Fallback ordering update is coherent.Including
"perplexity"in the OpenAI-compatible fallback list matches the new provider integration.providers/perplexity/provider.go (1)
12-25: Provider composition is clean and consistent.The constructor wires all required provider components correctly for OpenAI-compatible request/response handling.
providers/perplexity/provider_test.go (1)
10-79: Core provider test coverage is strong.Constructor wiring, URL resolution, invalid URL handling, and auth/content-type header behavior are well covered.
Summary
https://api.perplexity.aiPERPLEXITY_API_KEYenvironment variableSummary by CodeRabbit
New Features
Dependencies